CI Test Results

Run: #43267243603 | Commit: ff9a60e | Duration: 21m 8s (longest job)

✅ All 32 test jobs passed

Status Overview

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0

Updated: 2026-11-12 17:38:23 UTC

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 51338065

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Scan-Build Report

Bug Summary

Filter Results by File

Reports

Review findings (static review of the diff)

This review read the diff only. It did not build or run anything. Please verify each point before acting.

Correctness and robustness

1. mallocInfo.cpp:175 – the mallinfo2 mirror struct is not compiler-checked on el8.
   mallinfo2 is called through a hand-written function-pointer type and a mirror struct. The static_asserts that pin the mirror to glibc's struct sit inside #if __GLIBC_PREREQ(2,33). They do not compile on the el8 release image, which is the build this code targets. If glibc extends struct mallinfo2, the el8 binary would use a wrong layout. For a struct returned by value, this can mean a wrong return-buffer size and a stack overwrite.

2. mallocInfo.cpp:179 – dlsym(RTLD_DEFAULT, "mallinfo2") may not resolve to glibc.
   It returns the first match in global scope. A preloaded allocator can export its own mallinfo2 with a different layout. glibcMallocActive() only probes mallctl and tc_malloc_size, so it can miss such an allocator. The profiler would then read the result with glibc's layout and publish wrong values.

3. mallocInfo.cpp:140 – the pinning checks depend on include order.
   #include <malloc.h>, <stddef.h> and the static_asserts sit mid-file inside nested conditions. __GLIBC__ is defined only after a libc header is included. If <dlfcn.h> stops pulling in features.h, the guard turns false and the checks are skipped without a diagnostic.

Efficiency

4. flightRecorder.cpp:2161 – dlsym on every JFR chunk flush.
   Each flush does one dlsym lookup, and glibcMallocActive() adds two more. That is three loader-lock acquisitions per flush. The pointer does not change after startup. A function-local static or std::call_once would resolve it once.

Tests

5. MallocArenaCountersTest.java:685 – ldd --version can report the wrong glibc.
   ldd reports the system default glibc, not necessarily the one the JVM loaded. In a container or chroot, or with a bundled sysroot, the test can assert the wrong expectation. If ldd is missing, ProcessBuilder.start throws IOException and the test errors instead of skipping.

6. mallocArenaStats_ut.cpp:256 – new #error on unknown libc.
   A Linux gtest build that defines neither __GLIBC__ nor -D__musl__ now fails to compile. Before, those tests were compiled out. This matters for any build that does not go through GtestTaskBuilder.

7. mallocArenaStats_ut.cpp:284 – MmappedChunkIsReportedInHblkhdOnly is fragile.

   • It mallocs 256 MiB and expects an mmap-served chunk. Under RLIMIT_AS, a cgroup limit or disabled overcommit, the malloc fails and the test skips, which can hide regressions.
   • Other gtest threads or a lowered mmap threshold can break the uordblks/arena deltas.
   • The exact-equality assert arena == uordblks + fordblks compares separate snapshots with no stop-the-world.

@zhengyu123 Thanks — verified each point against the code. Three were real and are fixed in 64a395a / 5133806; the rest I believe are covered by the code as written, with the reasoning below.

Fixed:

3. Include order — mallocInfo.cpp now includes <features.h> explicitly on Linux, before the guard, so the pins no longer depend on <dlfcn.h> transitively defining __GLIBC__/__GLIBC_PREREQ.
4. dlsym per flush — both mallocArenaSnapshot() and glibcMallocActive() now resolve once into zero-initialized statics, published with acquire/release (__atomic_*, matching the repo's arm64 ordering policy). No loader-lock acquisition per flush; a racing first caller can only repeat the same lookup.
5. ldd probe — runtimeGlibcVersion() no longer throws on probe failure: missing ldd, timeout, interruption, and unparsable banners all skip via a JUnit assumption instead of erroring or asserting the wrong branch. The trailing-token regex now requires a token boundary, so a 2.38.9000-style banner is inconclusive rather than mis-parsed. The system-glibc assumption (ldd's glibc == the libc the JVM loaded) holds in every supported test image and is documented in the javadoc.

Not changed, with reasoning:

1. Mirror unchecked on el8 — on el8 headers struct mallinfo2 does not exist, so there is nothing to pin against; the mirror is the definition there. The residual risk is glibc growing the struct under the same symbol, which would break the return-by-value ABI of every binary compiled against 2.33+ headers — glibc's ABI contract (new fields come with new symbols, as mallinfo→mallinfo2 did) is the actual protection, and it is the same contract every pre-2.33-compiled consumer relies on. All builds whose headers do declare the struct compile the field pins; the el8 path is cross-checked at runtime by the accounting-identity/hblkhd premise tests. This is documented in mallocInfo.cpp.
2. RTLD_DEFAULT may hit another library's mallinfo2 — exporting a symbol named mallinfo2 is itself a declaration of glibc-mallinfo2 ABI compatibility; a library shipping a different layout under that name is broken by contract. The two real-world replacement allocators (tcmalloc, jemalloc) are excluded by glibcMallocActive() and covered by the LD_PRELOAD gtest. Fetching libc's own symbol specifically would need dl_iterate_phdr + dlopen on the flush path — not worth it for advisory counters absent a concrete allocator that both evades the probes and exports the name.
3. #error on unknown libc — intentional tripwire, documented at the site: a Linux build that can classify neither glibc nor musl is misconfigured, and the silent alternative is trimArenas() compiling to a no-op with the trim premise tests vanishing. The error message names the fix (-D__musl__).
4. MmappedChunkIsReportedInHblkhdOnly — the 256 MiB request is always above the dynamic mmap-threshold cap (32 MiB on LP64), so it is always mmap-served; verified empirically on glibc 2.35 and 2.41 in both a fresh process and the full-suite sequence. The gtest binaries are single-threaded, and expectMallinfoInvariants checks a single snapshot, not deltas across snapshots. The malloc-failure path skips rather than fails deliberately: in an RLIMIT_AS/cgroup-constrained environment a hard fail would be routine flakiness; the skip prints its reason so it is visible in review.

The remaining mutation-adequacy note (sentinel comparisons in glibcMallocActive() are untested) is covered by the existing GlibcMallocActive.ReportsActiveWithNoReplacementAllocatorLoaded / ReportsInactiveUnderSanitizerAllocator gtests, which call Recording::glibcMallocActive() via RecordingTestAccessor and assert both verdict directions.

This review also caught a real bug I had introduced while fixing an earlier one (the first caching version used plain non-atomic statics — formally UB on concurrent flushes); fixed with acquire/release publication in 5133806.

/merge

View all feedbacks in Devflow UI.

2026-11-12 15:43:49 UTC ℹ️ Start processing command /merge

2026-11-12 15:43:54 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 53m (p90).

2026-11-12 16:42:59 UTC ℹ️ MergeQueue: Readding this merge request to the queue because another merge request processed with yours failed. No action is needed from your side.

2026-11-12 17:14:08 UTC ℹ️ MergeQueue: This merge request was merged